Skip to content

SF-3603 Convert to angular standalone components#3535

Merged
Nateowami merged 42 commits intomasterfrom
task/convert-to-angular-standalone-components
Oct 31, 2025
Merged

SF-3603 Convert to angular standalone components#3535
Nateowami merged 42 commits intomasterfrom
task/convert-to-angular-standalone-components

Conversation

@siltomato
Copy link
Collaborator

@siltomato siltomato commented Oct 23, 2025

This PR refactors the app to use standalone components and modern Angular provider patterns. Also AppModule and main.ts are migrated to the new Angular standalone bootstrapping API.

Used migration script ng g @angular/core:standalone with all three modes:

  1. Convert declarations to standalone.
  2. Remove unnecessary NgModules.
  3. Switch to standalone bootstrapping API.

Afterwards, manual migrations were needed, following these principles:

Provider functions over Modules

  • Used provideX() functions returning Provider[] or EnvironmentProviders instead of Angular modules (@NgModule)
  • Examples: provideI18nStory(), provideRouter(), provideTestRealtime()

Individual Directive/Pipe imports

  • Replaced CommonModule imports with specific directives: AsyncPipe, NgClass, KeyValuePipe (import only what's actually used in the component template)

Routing configuration

  • Used provideRouter(routes) in providers array instead of RouterModule.forRoot() or RouterModule.forChild() in imports

Test Module wrapper elimination

  • Removed wrapper @NgModule test classes (e.g., TestModule, DialogTestModule)
  • Imported dependencies directly in configureTestingModule() imports array

This change is Reviewable

@siltomato siltomato added the will require testing PR should not be merged until testers confirm testing is complete label Oct 23, 2025
@Nateowami Nateowami added the e2e Run e2e tests for this pull request label Oct 23, 2025
@codecov
Copy link

codecov bot commented Oct 23, 2025

Codecov Report

❌ Patch coverage is 90.00000% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 82.47%. Comparing base (6c0ebdf) to head (836e65f).
⚠️ Report is 1 commits behind head on master.
✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
...te/editor/lynx/insights/lynx-insights-providers.ts 71.42% 2 Missing ⚠️
...draft-apply-dialog/draft-apply-dialog.component.ts 87.50% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #3535      +/-   ##
==========================================
+ Coverage   82.21%   82.47%   +0.25%     
==========================================
  Files         615      605      -10     
  Lines       37015    36866     -149     
  Branches     6060     6043      -17     
==========================================
- Hits        30433    30406      -27     
+ Misses       5678     5568     -110     
+ Partials      904      892      -12     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@siltomato siltomato force-pushed the task/convert-to-angular-standalone-components branch from 1346006 to 915e185 Compare October 23, 2025 17:49
@siltomato siltomato force-pushed the task/convert-to-angular-standalone-components branch from 915e185 to 18ec72f Compare October 24, 2025 15:31
@siltomato siltomato marked this pull request as ready for review October 24, 2025 22:35
@siltomato siltomato force-pushed the task/convert-to-angular-standalone-components branch from 6e057b5 to 7c59b63 Compare October 27, 2025 18:16
@Nateowami Nateowami requested a review from Copilot October 27, 2025 20:57
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

@pmachapman pmachapman self-requested a review October 27, 2025 22:05
Copy link
Collaborator

@pmachapman pmachapman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This review has turned out to be more epic than I initially thought! I'm submitting a partial review of what I have so far.

Also, I noticed that these two files need to be updated to remove standalone: true and standalone: false:

  • src/SIL.XForge.Scripture/ClientApp/src/app/shared/json-viewer/json-viewer.component.ts
  • src/SIL.XForge.Scripture/ClientApp/src/app/shared/json-viewer/json-viewer.component.spec.ts

@pmachapman reviewed 203 of 305 files at r1, all commit messages.
Reviewable status: 203 of 305 files reviewed, 4 unresolved discussions


src/SIL.XForge.Scripture/ClientApp/src/app/my-projects/my-projects.stories.ts line 195 at r1 (raw file):

      imports: [
        UICommonModule,
        SharedModule.forRoot(),

I think that the change to the chromatic tests is from this removal. (I don't think we should add provideUICommon() - I just wanted to flag what I think the possible cause is)

Code quote:

        UICommonModule,
        SharedModule.forRoot(),

src/SIL.XForge.Scripture/ClientApp/src/app/serval-administration/job-events-dialog.component.ts line 39 at r1 (raw file):

  imports: [MatDialogTitle, MatDialogContent, MatDialogActions, MatDialogClose, MatIcon, JsonViewerComponent]
})
export class JobEventsDialogComponent {

RE: Your merge conflict, this class is now JobDetailsDialogComponent (job-details-dialog.component.ts)

Code quote:

export class JobEventsDialogComponent {

src/SIL.XForge.Scripture/ClientApp/src/app/event-metrics/event-metric-dialog.component.html line 2 at r1 (raw file):

<ng-container *transloco="let t; read: 'event_metric_dialog'">
  <div [dir]="i18n.direction">

I'm guessing this was changed because <mat-dialog> is not a valid tag?

I did a quick search and this tag is still being used in:

  • src/SIL.XForge.Scripture/ClientApp/src/app/checking/import-questions-dialog/import-questions-confirmation-dialog/import-questions-confirmation-dialog.component.html
  • src/SIL.XForge.Scripture/ClientApp/src/app/settings/delete-project-dialog/delete-project-dialog.component.html

Code quote:

<div [dir]="i18n.direction">

src/SIL.XForge.Scripture/ClientApp/.storybook/preview.ts line 13 at r1 (raw file):

import docJson from '../documentation.json';
import { provideSFTabs } from '../src/app/shared/sf-tab-group';
import { provideUICommon } from '../src/xforge-common/ui-common-providers';

NIT: This should be:

import { provideUICommon } from 'xforge-common/ui-common-providers';

Code quote:

import { provideUICommon } from '../src/xforge-common/ui-common-providers';

src/SIL.XForge.Scripture/ClientApp/src/main.ts line 10 at r1 (raw file):

} from '@angular/core';

import { ExceptionHandlingService } from 'xforge-common/exception-handling.service';

NIT: If the new lines are removed from above and below this line, the imports will sort correctly.

Suggestion:

import { ExceptionHandlingService } from 'xforge-common/exception-handling.service';

@siltomato siltomato force-pushed the task/convert-to-angular-standalone-components branch from 7c59b63 to 96691a4 Compare October 28, 2025 17:09
Copy link
Collaborator Author

@siltomato siltomato left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewable status: 203 of 305 files reviewed, 2 unresolved discussions (waiting on @pmachapman)


src/SIL.XForge.Scripture/ClientApp/.storybook/preview.ts line 13 at r1 (raw file):

Previously, pmachapman (Peter Chapman) wrote…

NIT: This should be:

import { provideUICommon } from 'xforge-common/ui-common-providers';

Done.


src/SIL.XForge.Scripture/ClientApp/src/main.ts line 10 at r1 (raw file):

Previously, pmachapman (Peter Chapman) wrote…

NIT: If the new lines are removed from above and below this line, the imports will sort correctly.

Done.


src/SIL.XForge.Scripture/ClientApp/src/app/event-metrics/event-metric-dialog.component.html line 2 at r1 (raw file):

Previously, pmachapman (Peter Chapman) wrote…

I'm guessing this was changed because <mat-dialog> is not a valid tag?

I did a quick search and this tag is still being used in:

  • src/SIL.XForge.Scripture/ClientApp/src/app/checking/import-questions-dialog/import-questions-confirmation-dialog/import-questions-confirmation-dialog.component.html
  • src/SIL.XForge.Scripture/ClientApp/src/app/settings/delete-project-dialog/delete-project-dialog.component.html

Good catch, thanks!


src/SIL.XForge.Scripture/ClientApp/src/app/my-projects/my-projects.stories.ts line 195 at r1 (raw file):

Previously, pmachapman (Peter Chapman) wrote…

I think that the change to the chromatic tests is from this removal. (I don't think we should add provideUICommon() - I just wanted to flag what I think the possible cause is)

Yeah, maybe it is related to removal of UICommonModule, although provideUICommon() is now being applied to all stories via preview.ts.


src/SIL.XForge.Scripture/ClientApp/src/app/serval-administration/job-events-dialog.component.ts line 39 at r1 (raw file):

Previously, pmachapman (Peter Chapman) wrote…

RE: Your merge conflict, this class is now JobDetailsDialogComponent (job-details-dialog.component.ts)

Thanks for the heads up!

Copy link
Collaborator

@pmachapman pmachapman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a couple more nits!

@pmachapman reviewed 102 of 305 files at r1, 10 of 10 files at r2, all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @siltomato)


src/SIL.XForge.Scripture/ClientApp/src/app/translate/training-progress/training-progress.component.ts line 11 at r2 (raw file):

import { quietTakeUntilDestroyed } from 'xforge-common/util/rxjs-util';

import { MatIconButton } from '@angular/material/button';

NIT: Are you able to remove the line break here to help sorting?

Code quote:

import { quietTakeUntilDestroyed } from 'xforge-common/util/rxjs-util';

import { MatIconButton } from '@angular/material/button';

src/SIL.XForge.Scripture/ClientApp/src/app/translate/editor/tabs/editor-tab-add-resource-dialog/editor-tab-add-resource-dialog.component.spec.ts line 5 at r2 (raw file):

import { provideAnimations } from '@angular/platform-browser/animations';

import { SFProject } from 'realtime-server/lib/esm/scriptureforge/models/sf-project';

NIT: Are you able to remove the line break here to help sorting?

Code quote:

import { provideAnimations } from '@angular/platform-browser/animations';

import { SFProject } from 'realtime-server/lib/esm/scriptureforge/models/sf-project';

@pmachapman pmachapman self-assigned this Oct 29, 2025
@siltomato siltomato force-pushed the task/convert-to-angular-standalone-components branch 2 times, most recently from d1d6b39 to e91fdf2 Compare October 29, 2025 15:08
Copy link
Collaborator Author

@siltomato siltomato left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewable status: 291 of 310 files reviewed, all discussions resolved (waiting on @pmachapman)


src/SIL.XForge.Scripture/ClientApp/src/app/translate/editor/tabs/editor-tab-add-resource-dialog/editor-tab-add-resource-dialog.component.spec.ts line 5 at r2 (raw file):

Previously, pmachapman (Peter Chapman) wrote…

NIT: Are you able to remove the line break here to help sorting?

Done.


src/SIL.XForge.Scripture/ClientApp/src/app/translate/training-progress/training-progress.component.ts line 11 at r2 (raw file):

Previously, pmachapman (Peter Chapman) wrote…

NIT: Are you able to remove the line break here to help sorting?

Done.

Copy link
Collaborator

@pmachapman pmachapman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:lgtm:

@pmachapman reviewed 21 of 21 files at r3, all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @siltomato)

@siltomato siltomato force-pushed the task/convert-to-angular-standalone-components branch from d50c032 to 836e65f Compare October 30, 2025 19:08
@Nateowami Nateowami merged commit 1409d53 into master Oct 31, 2025
22 of 23 checks passed
@Nateowami Nateowami deleted the task/convert-to-angular-standalone-components branch October 31, 2025 13:03
@Nateowami Nateowami added testing complete Testing of PR is complete and should no longer hold up merging of the PR and removed ready to test labels Oct 31, 2025
@Nateowami Nateowami restored the task/convert-to-angular-standalone-components branch March 11, 2026 02:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

e2e Run e2e tests for this pull request testing complete Testing of PR is complete and should no longer hold up merging of the PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants